Implement parsing of pinned borrows#135731
Conversation
|
rustbot has assigned @petrochenkov. Use |
|
Some changes occurred to the CTFE machinery cc @rust-lang/wg-const-eval |
This comment has been minimized.
This comment has been minimized.
ceaa211 to
e57789f
Compare
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
|
r? compiler |
This is not the semantic we want or a good placeholder for it. What we want is that fn f<T: Unpin>(x: &mut T) {
let _: Pin<&mut T> = &pin mut *x; // i.e., `Pin::new(&mut *x)`.
}That is, if the type in the place is After that, we start talking about how to extend this to safe pin projection to struct fields. |
|
r? traviscross |
I agree with you, and my previous statement was not precise. What I mean is similar to yours. I expect The pinned places might be defined as: A place is a pinned place if one of the following conditions holds:
Here are a few examples to explain: #[derive(Default)]
struct Foo<T> {
x: T,
}
fn f<T: Default, U: Default + Unpin>() {
// Case 1: not `Unpin`, and not pinned
let mut a = Foo::<T>::default(); // `a` and `a.x` are non-pinned places but not pinned places
let _: &mut Foo = &mut a; // ok
let _: &mut T = &mut a.x; // ok
let _: Pin<&mut Foo> = &pin mut a; // ERROR
let _: Pin<&mut T> = &pin mut a.x; // ERROR
// Case 2: not `Unpin`, and pinned
let pin mut b = Foo::<T>::default(); // `b` and `b.x` are pinned places but not non-pinned places
let _: &mut Foo = &mut b; // ERROR
let _: &mut Foo = &mut b.x; // ERROR
let _: &pin mut Foo = &pin mut b; // ok
let _: &pin mut Foo = &pin mut b.x; // ok
// Case 3: `Unpin`, and not pinned
let mut c = Foo::<U>::default(); // `c` and `c.x` are both pinned and non-pinned places
let _: &mut Foo = &mut c; // ok
let _: &mut T = &mut c.x; // ok
let _: Pin<&mut Foo> = &pin mut c; // ok
let _: Pin<&mut T> = &pin mut c.x; // ok
// Case 4: `Unpin`, and pinned
let pin mut d = Foo::<U>::default(); // WARN (useless `pin`) // `d` and `d.x` are both pinned and non-pinned places
let _: &mut Foo = &mut d; // ok
let _: &mut T = &mut d.x; // ok
let _: Pin<&mut Foo> = &pin mut d; // ok (useless `pin`)
let _: Pin<&mut T> = &pin mut d.x; // ok (useless `pin`)
} |
|
Thanks, that's helpful. Going through the examples, starting on Case 1, we're actually instead interested in this working: struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
let _: Pin<&mut Foo<T>> = &pin mut x; //~ OK
let _: &Foo<T> = &x; //~ OK
let _: &mut Foo<T> = &mut x; //~ ERROR place has been pinned
let _move: Foo<T> = x; //~ ERROR place has been pinned
}The In this way, This is also why we're not sold on doing Regarding the safe pin projections to fields, let's do that in a separate PR so we can set those questions aside for the moment. That a place is pinned doesn't necessarily imply that we can safely pin project to a field without taking a number of other steps. |
|
Thanks for your feedback too! Regarding your code snippet, I have some questions. First, this is the semantics of struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
let _: Pin<&mut Foo<T>> = pin!(x); //~ OK
let _: &Foo<T> = &x; //~ ERROR place has been moved
let _: &mut Foo<T> = &mut x; //~ ERROR place has been moved
let _move: Foo<T> = x; //~ ERROR place has been moved
}Then in your version,
What I expect is, the borrow checker should allow the original place to be mutably borrowed or moved after all pinned borrows expired (if we don't introduce fn ignore<T>(_: T) {}
struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
{
let _pinned: Pin<&mut Foo<T>> = &pin mut x; //~ OK
let _: &Foo<T> = &x; //~ OK
// Mutable borrows or moves are not allowed during the pinned borrow is alive
let _: &mut Foo<T> = &mut x; //~ ERROR place has been pinned
let _move: Foo<T> = x; //~ ERROR place has been pinned
// Make sure `_pinned` is used
ignore(_pinned);
}
let _: &Foo<T> = &x; //~ OK
// Mutable borrows or moves are allowed again after the pinned borrow expires
let _: &mut Foo<T> = &mut x; //~ OK
let _move: Foo<T> = x; //~ OK
} |
e57789f to
1c081ea
Compare
This can't be allowed. The contract with What the borrow checker must do here, then, is to flag the struct Foo<T> { x: T }
fn f<T>(mut x: Foo<T>) {
let _: &mut Foo<T> = &mut x; //~ OK
//~^ Before we pin the thing, it's OK for mutable references to
//~| exist to it.
//~|
//~| Note that the above mutable reference is dead at this point.
let _: Pin<&mut Foo<T>> = &pin mut x; //~ OK
//~^ Now we create a pinned mutable reference. This is allowed
//~| since the earlier mutable reference is dead. Once this
//~| pinned reference is created, we can no longer allow the
//~| pointed to memory to be invalidated or repurposed until
//~| after `drop` has been run, even after this pinned reference
//~| is dead.
//~|
//~| Note that the pinned reference is dead at this point.
let _: &Foo<T> = &x; //~ OK
//~^ We can allow shared references because they don't allow us
//~| to invalidate or repurpose the memory.
let _: Pin<&mut Foo<T>> = &pin mut x; //~ OK
//~^ We can allow a new pinned mutable reference for the same
//~| reason.
let _: Pin<&Foo<T>> = &pin const x; //~ OK
//~^ Or a new pinned shared reference, for that matter.
let _: &mut Foo<T> = &mut x; //~ ERROR place has been pinned
//~^ We can't allow a mutable reference to exist, though, because
//~| that would violate the contract.
let _move: Foo<T> = x; //~ ERROR place has been pinned
//~^ Neither can we allow the once-pinned thing to be moved.
}What the |
Oh, yes, I forgot that |
|
Yes, the intuition point is something we'll I'm sure consider later. We want to try it first in the way mentioned. |
|
What @traviscross said above sounds right to me. I would also mention that we want dispatch on |
|
☔ The latest upstream changes (presumably #138114) made this pull request unmergeable. Please resolve the merge conflicts. |
…holk,traviscross Implement parsing of pinned borrows This PR implements part of rust-lang#130494. EDIT: It introduces `&pin mut $place` and `&pin const $place` as sugars for `std::pin::pin!($place)` and its shared reference equivalent, except that `$place` will not be moved when borrowing. The borrow check will be in charge of enforcing places cannot be moved or mutably borrowed since being pinned till dropped. ### Implementation steps: - [x] parse the `&pin mut $place` and `&pin const $place` syntaxes - [ ] borrowck of `&pin mut|const` - [ ] support autoref of `&pin mut|const` when needed
Rollup of 15 pull requests Successful merges: - #135731 (Implement parsing of pinned borrows) - #138780 (Add `#[loop_match]` for improved DFA codegen) - #142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - #142633 (Error on invalid signatures for interrupt ABIs) - #142768 (Avoid a bitcast FFI call in transmuting) - #142825 (Port `#[track_caller]` to the new attribute system) - #142844 (Enable short-ice for Windows) - #142934 (Tweak `-Zmacro-stats` measurement.) - #142955 (Couple of test suite fixes for cg_clif) - #142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - #142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - #142982 (Corrected spelling mistake in c_str.rs) - #142983 (Taint body on invalid call ABI) - #142988 (Update wasm-component-ld to 0.5.14) - #142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #135731 - frank-king:feature/pin-borrow, r=eholk,traviscross Implement parsing of pinned borrows This PR implements part of #130494. EDIT: It introduces `&pin mut $place` and `&pin const $place` as sugars for `std::pin::pin!($place)` and its shared reference equivalent, except that `$place` will not be moved when borrowing. The borrow check will be in charge of enforcing places cannot be moved or mutably borrowed since being pinned till dropped. ### Implementation steps: - [x] parse the `&pin mut $place` and `&pin const $place` syntaxes - [ ] borrowck of `&pin mut|const` - [ ] support autoref of `&pin mut|const` when needed
Rollup of 15 pull requests Successful merges: - rust-lang/rust#135731 (Implement parsing of pinned borrows) - rust-lang/rust#138780 (Add `#[loop_match]` for improved DFA codegen) - rust-lang/rust#142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - rust-lang/rust#142633 (Error on invalid signatures for interrupt ABIs) - rust-lang/rust#142768 (Avoid a bitcast FFI call in transmuting) - rust-lang/rust#142825 (Port `#[track_caller]` to the new attribute system) - rust-lang/rust#142844 (Enable short-ice for Windows) - rust-lang/rust#142934 (Tweak `-Zmacro-stats` measurement.) - rust-lang/rust#142955 (Couple of test suite fixes for cg_clif) - rust-lang/rust#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - rust-lang/rust#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - rust-lang/rust#142982 (Corrected spelling mistake in c_str.rs) - rust-lang/rust#142983 (Taint body on invalid call ABI) - rust-lang/rust#142988 (Update wasm-component-ld to 0.5.14) - rust-lang/rust#142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
…kingjubilee Rollup of 15 pull requests Successful merges: - rust-lang#135731 (Implement parsing of pinned borrows) - rust-lang#138780 (Add `#[loop_match]` for improved DFA codegen) - rust-lang#142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - rust-lang#142633 (Error on invalid signatures for interrupt ABIs) - rust-lang#142768 (Avoid a bitcast FFI call in transmuting) - rust-lang#142825 (Port `#[track_caller]` to the new attribute system) - rust-lang#142844 (Enable short-ice for Windows) - rust-lang#142934 (Tweak `-Zmacro-stats` measurement.) - rust-lang#142955 (Couple of test suite fixes for cg_clif) - rust-lang#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - rust-lang#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - rust-lang#142982 (Corrected spelling mistake in c_str.rs) - rust-lang#142983 (Taint body on invalid call ABI) - rust-lang#142988 (Update wasm-component-ld to 0.5.14) - rust-lang#142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
…raviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of #130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (#135731).~ CC `@traviscross`
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC `@traviscross`
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC `@traviscross`
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC ``@traviscross``
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC ```@traviscross```
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC ````@traviscross````
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC `````@traviscross`````
…Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang#135731).~ CC ``````@traviscross``````
Rollup merge of #139751 - frank-king:feature/pin-project, r=Nadrieril,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of #130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (#135731).~ CC ``````@traviscross``````
…,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang/rust#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang/rust#135731).~ CC ``````@traviscross``````
…,traviscross Implement pin-project in pattern matching for `&pin mut|const T` This PR implements part of rust-lang/rust#130494. It supports pin-project in pattern matching for `&pin mut|const T`. ~Pin-projection by field access (i.e. `&pin mut|const place.field`) is not fully supported yet since pinned-borrow is not ready (rust-lang/rust#135731).~ CC ``````@traviscross``````
| let mut arg = self.mirror_expr(arg_expr); | ||
| // For `&pin mut $place` where `$place` is not `Unpin`, move the place | ||
| // `$place` to ensure it will not be used afterwards. | ||
| if mutbl.is_mut() && !arg_ty.is_unpin(self.tcx, self.typing_env) { |
There was a problem hiding this comment.
FWIW, using is_unpin here is quite questionable as that is a safe trait and hence cannot have much semantic relevance. I added this as an open question to the tracking issue.
There was a problem hiding this comment.
It's a working around.
By design, &pin borrows should be checked by the borrow checker that since a place has been pinned borrow, it should not be moved and should not be mutably borrowed (i.e. &mut) until the value is destructed.
The checking process is not implemented yet, so this just a work around to avoid the soundness issue.
Rollup of 15 pull requests Successful merges: - rust-lang/rust#135731 (Implement parsing of pinned borrows) - rust-lang/rust#138780 (Add `#[loop_match]` for improved DFA codegen) - rust-lang/rust#142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - rust-lang/rust#142633 (Error on invalid signatures for interrupt ABIs) - rust-lang/rust#142768 (Avoid a bitcast FFI call in transmuting) - rust-lang/rust#142825 (Port `#[track_caller]` to the new attribute system) - rust-lang/rust#142844 (Enable short-ice for Windows) - rust-lang/rust#142934 (Tweak `-Zmacro-stats` measurement.) - rust-lang/rust#142955 (Couple of test suite fixes for cg_clif) - rust-lang/rust#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - rust-lang/rust#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - rust-lang/rust#142982 (Corrected spelling mistake in c_str.rs) - rust-lang/rust#142983 (Taint body on invalid call ABI) - rust-lang/rust#142988 (Update wasm-component-ld to 0.5.14) - rust-lang/rust#142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 15 pull requests Successful merges: - rust-lang/rust#135731 (Implement parsing of pinned borrows) - rust-lang/rust#138780 (Add `#[loop_match]` for improved DFA codegen) - rust-lang/rust#142453 (Windows: make `read_dir` stop iterating after the first error is encountered) - rust-lang/rust#142633 (Error on invalid signatures for interrupt ABIs) - rust-lang/rust#142768 (Avoid a bitcast FFI call in transmuting) - rust-lang/rust#142825 (Port `#[track_caller]` to the new attribute system) - rust-lang/rust#142844 (Enable short-ice for Windows) - rust-lang/rust#142934 (Tweak `-Zmacro-stats` measurement.) - rust-lang/rust#142955 (Couple of test suite fixes for cg_clif) - rust-lang/rust#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠) - rust-lang/rust#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect) - rust-lang/rust#142982 (Corrected spelling mistake in c_str.rs) - rust-lang/rust#142983 (Taint body on invalid call ABI) - rust-lang/rust#142988 (Update wasm-component-ld to 0.5.14) - rust-lang/rust#142993 (Update cargo) r? `@ghost` `@rustbot` modify labels: rollup
This PR implements part of #130494.
EDIT: It introduces
&pin mut $placeand&pin const $placeas sugars forstd::pin::pin!($place)and its shared reference equivalent, except that$placewill not be moved when borrowing. The borrow check will be in charge of enforcing places cannot be moved or mutably borrowed since being pinned till dropped.Implementation steps:
&pin mut $placeand&pin const $placesyntaxes&pin mut|const&pin mut|constwhen needed